Skip to content

Liuliu/log-graph#8161

Merged
liuliu-dev merged 19 commits intomainfrom
liuliu/log-graph
Aug 2, 2024
Merged

Liuliu/log-graph#8161
liuliu-dev merged 19 commits intomainfrom
liuliu/log-graph

Conversation

@liuliu-dev
Copy link
Copy Markdown
Contributor

@liuliu-dev liuliu-dev commented Jul 26, 2024 •
edited
Loading

command: dolt log --graph

Steps for Drawing the Commit Graph

  1. Calculate the Positions of Each Commit:

  • Row Position: Determined by the commit's topological order, initialized using the index in the list. Adjust the vertical spacing between commits based on the commit message's height in expandGraph function.

  • Column Position, calculated based on the positions of child commits, implemented in computeColumnEnds function:

    • No Children: Places the commit in a new column (furthest right) if it has no child commits, indicating the start of a new branch.
    • Branch Children: If a commit has branch children, it is positioned in the same column as its leftmost branch child.
    • Merge Children: For commits with merge children, diagonal lines ('') are used to connect to these children, positioning the commit in an available column starting from its rightmost child.
  1. Render the Graph:
    Place a * at each commit's calculated position to mark it.
    Connect commits using vertical (|) and diagonal (\ or /) lines to represent branches and merges.

An example of the graph of us-jails:

Screenshot 2024-08-02 at 10 11 12 AM

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 26, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
af879e6 ok 5937457
version total_tests
af879e6 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 26, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
c23f150 ok 5937457
version total_tests
c23f150 5937457
correctness_percentage
100.0

@liuliu-dev liuliu-dev requested a review from macneale4 July 29, 2024 19:38
@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 29, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
16c683e ok 5937457
version total_tests
16c683e 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 29, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
5695942 ok 5937457
version total_tests
5695942 5937457
correctness_percentage
100.0

@timsehn
Copy link
Copy Markdown
Contributor

timsehn commented Jul 30, 2024

A couple of output graphs in code blocks would be cool to see.

timsehn
timsehn reviewed Jul 30, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@timsehn timsehn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably needs a couple bats tests.

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 31, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
5ce04ab ok 5937457
version total_tests
5ce04ab 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 31, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
c924c8c ok 5937457
version total_tests
c924c8c 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 31, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
abacfb2 ok 5937457
version total_tests
abacfb2 5937457
correctness_percentage
100.0

macneale4
macneale4 requested changes Jul 31, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm realizing now that I need a higher level description of the algorithm in order to properly review this. I want you to be overly heavy on documentation of fields and functions. Comments in functions are almost always a sign that the function is too complex, and should be broken into multiple functions. Not always, just a rule of thumb. I'm seeing some pretty long functions in this code, and the descriptions of what they each do doesn't come close to the amount of code in the function.

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Jul 31, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
a0a8dab ok 5937457
version total_tests
a0a8dab 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Aug 1, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
a5f41cc ok 5937457
version total_tests
a5f41cc 5937457
correctness_percentage
100.0

macneale4
macneale4 requested changes Aug 1, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming together! I think one more rev and you'll be good to go

go/cmd/dolt/commands/log_graph.go Outdated
*
*/

type CommitInfoWithChildren struct {
Copy link
Copy Markdown
Contributor

@macneale4 macneale4 Aug 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type is only used in the package, so make it start with a lower case letter (makes it package private)

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Aug 1, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
e08f120 ok 5937457
version total_tests
e08f120 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Aug 2, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
154d90c ok 5937457
version total_tests
154d90c 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Aug 2, 2024

@liuliu-dev DOLT

comparing_percentages
100.000000 to 100.000000
version result total
2382bb3 ok 5937457
version total_tests
2382bb3 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

coffeegoddd commented Aug 2, 2024

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
eed2184 ok 5937457
version total_tests
eed2184 5937457
correctness_percentage
100.0

macneale4
macneale4 approved these changes Aug 2, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

     .  o ..                  
     o . o o.o                
          ...oo               
            __[]__            
         __|_o_o_o\__         
         \""""""""""/         
          \. ..  . /          
     ^^^^^^^^^^^^^^^^^^^^

@liuliu-dev liuliu-dev merged commit 2dbb49c into main Aug 2, 2024
@liuliu-dev liuliu-dev deleted the liuliu/log-graph branch August 2, 2024 20:12
@BrewTestBot BrewTestBot mentioned this pull request Aug 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timsehn timsehn timsehn left review comments

@macneale4 macneale4 macneale4 approved these changes

Assignees

No one assigned

Labels

correctness_approved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@liuliu-dev @coffeegoddd @timsehn @macneale4